Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inject defaultCatalog property to chaise-config and remove catalog alias creation step #2497

Merged
merged 2 commits into from
Aug 2, 2024

Conversation

RFSH
Copy link
Member

@RFSH RFSH commented Aug 1, 2024

Summary

A while ago, I modified the catalog creation step of our test framework to add aliases to the created catalog. This allowed us to test the defaultCatalog property since we could hardcode the alias inside the chaise-config.js files.

This worked properly while I was testing it on my own. However, since then, we have found that the alias creation has limitations when multiple users use the same alias or if multiple users want to run test cases simultaneously.

So this PR will remove the alias creation step and instead will inject a line into the chaise-config.js like the following:

chaiseConfig.defaultCatalog = 'id_of_the_newly_created_catalog';

Since we don't need this for all the test cases, I added a new addDefaultCatalogToChaiseConfig to our test options.

Details

The following are the issues that we found with the alias method:

  1. Since an alias can be associated with just one catalog at a time, running multiple instances of our test framework simultaneously could result in unpredicted behavior. For example, assume user1 runs the test cases and assigns my-alias to a catalog. At the same time, user2 runs the test case and assigns my-alias to their own newly created catalog. In this case, using the my-alias will refer to one of these catalogs, so one of the users is using the wrong catalog.
  2. If we don't provide the owner property while creating the alias, whoever creates it first will be its sole owner. So, if someone else attempts to use the same alias for a different catalog, they will get an error. To solve this, we would have to provide the isrd-testers as the owner of the alias. But that means that we have to hardcode this as part of chaise.

…ias creation step

while the alias method worked properly for one user, we found several issues when multiple
people want to run the test cases:

1. when some one adds alias to a catalog, they will become the owner of that alias. so the next person
   won't be able to use the same alias on a different catalog. We might be able to solve this by passing
   the owner property and use the isrd-testers or isrd-gorups. but then we have to hardcode this in our
   chaise test step.

2. we are not able to run multiple instances of our test suite at the same time on the same server. so for example
   if user1 and user2 start running the test cases together, since we're using the same alias name in these two
   instances, the alias is going to be attached to only one of the catalogs which can cause issues.
@RFSH RFSH requested a review from jrchudy August 1, 2024 22:33
@RFSH RFSH self-assigned this Aug 1, 2024
Copy link
Member

@jrchudy jrchudy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand what you mean as "hacky" because you have to read the file into memory, add to it, and then write it back since this is "before" the javascript test environment starts running.

@RFSH RFSH merged commit b58ece4 into master Aug 2, 2024
1 check passed
@RFSH RFSH deleted the inject-default-catalog branch August 2, 2024 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants